Conversation
📝 WalkthroughWalkthroughAdds optional bearer-token authentication across CLI and API utilities: new CLI flag, extended option types, direct fetch bearer endpoints in tools (environments, playwright, test-cases), logger log-level helper, package version bump to 4.5.0, and tests covering both API-key and bearer-token flows. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Debugtopus as Debugtopus
participant Tools as Tools (playwright/environments/test-cases)
participant Client as API Client
participant Service as Backend Service
alt Bearer token provided
CLI->>Debugtopus: run debug with --bearer-token
Debugtopus->>Tools: call getPlaywrightConfig/getPlaywrightCode (bearerToken)
Tools->>Service: fetch BASE_URL/bearer/... with Authorization: Bearer <token>
Service-->>Tools: bearer response (text/json)
Tools-->>Debugtopus: return parsed config/code
else API key path
CLI->>Debugtopus: run debug without bearer token
Debugtopus->>Tools: call getPlaywrightConfig/getPlaywrightCode (apiKey)
Tools->>Client: client.GET /apiKey/v3/...
Client->>Service: request with apiKey
Service-->>Client: API response
Client-->>Tools: return parsed response
Tools-->>Debugtopus: return config/code
end
Debugtopus-->>CLI: config/code available
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tools/test-cases.ts`:
- Around line 129-143: The bearer-token branch for fetching test cases omits the
filter query that the API-key path includes; update the code inside the if
(options.bearerToken) block to append the same JSON-encoded query parameter used
by the API-key path (include options.status and options.runStatus) to the
request URL (keep using BASE_URL and options.testTargetId) before calling fetch,
and preserve the Authorization: `Bearer ${options.bearerToken}` header so the
endpoint receives the identical filter structure as in the API-key path.
🧹 Nitpick comments (5)
src/logger.ts (1)
54-58: Consider validating LOG_LEVEL environment variable.The cast
process.env.LOG_LEVEL as LogLevelaccepts any string at runtime. If an invalid log level is provided (e.g.,LOG_LEVEL=verbose), the behavior depends on the library's handling of unknown levels.💡 Suggested validation
+const validLogLevels = ["trace", "debug", "info", "warning", "error", "fatal"] as const; +const getLogLevel = (): LogLevel => { + const level = process.env.LOG_LEVEL; + if (level && validLogLevels.includes(level as LogLevel)) { + return level as LogLevel; + } + return "warning"; +}; + export const configureLogger = async (): Promise<void> => configure({ // ... loggers: [ // ... { category: "octomind", - lowestLevel: (process.env.LOG_LEVEL as LogLevel) || "warning", + lowestLevel: getLogLevel(), sinks: ["console"], }, ], });src/cli.ts (1)
152-155: Note:-bshorthand has different meanings across commands.The
debugcommand uses-bfor--bearer-token, while theexecutecommand (line 174) uses-bfor--browser. This is technically valid since they're separate commands, but could cause user confusion.Consider using a different shorthand (e.g.,
-Bor--bearer-tokenonly without shorthand) for consistency, or document this distinction clearly.src/tools/test-cases.ts (1)
138-142: Improve error handling for bearer token fetch.The current error handling only uses
res.statusTextwhich may not provide sufficient debugging information. Consider including the response body for non-ok responses.💡 Suggested improvement
if (res.ok) { const response = await res.json(); return response; } - throw new Error(`no test cases found. error: ${res.statusText}`); + let errorDetail = res.statusText; + try { + const errorBody = await res.text(); + if (errorBody) { + errorDetail = `${res.status} ${res.statusText}: ${errorBody}`; + } + } catch { + // Ignore if we can't read the body + } + throw new Error(`Failed to fetch test cases: ${errorDetail}`); }src/tools/environments.ts (1)
54-57: Add type assertion for consistency with other bearer token paths.The
res.json()call returns an untyped value. For consistency withsrc/tools/playwright.ts(which usesas PlaywrightCodeResponse), consider adding a type assertion here.♻️ Suggested improvement
if (res.ok) { - const environments = await res.json(); + const environments = (await res.json()) as EnvironmentResponse[]; return environments; }src/tools/playwright.ts (1)
17-29: Redundant conditional spreads for required fields.
urlandoutputDirare required in the function signature, so the conditional checksoptions.url &&andoptions.outputDir &&are unnecessary. Additionally, if these fields were empty strings (which satisfies the type but is falsy), they would be incorrectly omitted from the query params.♻️ Suggested improvement
const params = new URLSearchParams({ ...(options.environmentId && { environmentId: options.environmentId }), - ...(options.url && { url: options.url }), - ...(options.outputDir && { outputDir: options.outputDir }), + url: options.url, + outputDir: options.outputDir, ...(options.headless !== undefined && { headless: options.headless.toString(), }),
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.